Skip to content

Conversation

@AliSoftware
Copy link
Contributor

@AliSoftware AliSoftware commented Feb 16, 2021

This is the second attempt at #14080 as it seems the fix wasn't sufficient and was merged too soon (before we finished testing the fix on the other PRs)

  • The first build with the fix, on the PR fixing it, passed – and PR Invalidate bundler cache to fix CI failure #14080 went green.
  • But then the build failed on the other PRs I cherry-picked it on (which were triggered in parallel).
  • Hopefully that failure was only because the builds were racing each other as it seems like cache got picked by 2nd PR before 1st PR finished building it. So hopefully waiting until the job is done this time will help. I also used different cache keys for the different jobs just to be on the safe side. In fact the issue was deeper than this, see comments below and also complementary details in the WCAndroid PR doing the same on Woo

To test:

PR created as Draft to be sure to not merge it too fast this time

@AliSoftware AliSoftware added this to the 16.7 ❄️ milestone Feb 16, 2021
@AliSoftware AliSoftware requested a review from a team February 16, 2021 12:52
@AliSoftware AliSoftware self-assigned this Feb 16, 2021
@AliSoftware AliSoftware requested review from jkmassel, loremattei and oguzkocer and removed request for a team February 16, 2021 12:52
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Feb 16, 2021

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Feb 16, 2021

You can test the changes on this Pull Request by downloading the APK here.

@AliSoftware
Copy link
Contributor Author

After multiple attempts and debugging, it happens that the solution is to update bundler to 2.2.10.

This is because CircleCI has updated their image to use bundler 2.2.10 apparently recently, very likely because this version contains an important security fix about an issue that made some noise on internet recently.

Bundle bundler 2.2.10 also generates a different Gemfile.lock (to separate the gem in the lockfile by source as part of solving that security issue) and I'm guessing likely a slightly different resolution algorithm taking that into account, which is why the Gemfile.lock which was resolved by bundler 2.1.4 (that we were using up until now) was causing trouble to bundler 2.2.10.

@AliSoftware
Copy link
Contributor Author

AliSoftware commented Feb 16, 2021

After reading in more details the whole PR on bundler about this security issue, I realized that with the setup we had made bundler still use the multi-source mode, because the plugin we include in Pluginfile was not inside the source block so resolved its dependencies from a different set of sources.

Explanations of that side-effect issue

bundler's PR explains that the whole point of fixing the security issue is to also recommend to avoid using multiple sources in Gemfiles, and using source 'xxx' do … end blocks to group some gems in a given source and some others (aka the ones outside the block) not having the same source will also cause issues.

In fact, as discussed in this comment on the PR, when resolving transitive dependencies it will use the source of the parent gem, but in the case of our Pluginfile which doesn't specify a source, that only resolves to local gems, so wasn't able to find rake and octokit and others locally (when cache was empty at least), instead of sourcing them from rubygems.orgs like all other gems, and leading to the Gemfile.lock having multiple sources (hence the initially bigger diff on the Gemfile.lock after this change before the last commit)…

Only figured that out while trying to replicate the fix on WCAndroid in woocommerce/woocommerce-android#3585 – so now we use a single global source at the top, applied to all gems including the plugin.

Pfew 😓 I hope that's it this time

@AliSoftware AliSoftware changed the title Attempt to fix the CI bundler/rake cache failure again [Tooling] Fix the bundler/rake failure on CI Feb 16, 2021
@AliSoftware
Copy link
Contributor Author

- checkout-submodules
- bundle-install/bundle-install:
cache_key_prefix: installable-build-v2
cache_key_prefix: release-build-v2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

- checkout-submodules
- bundle-install/bundle-install:
cache_key_prefix: installable-build-v2
cache_key_prefix: translation-review-build-v2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice

@AliSoftware AliSoftware merged commit d9bfae6 into develop Feb 16, 2021
@AliSoftware AliSoftware deleted the fix/bundle-ci-cache-attempt2 branch February 16, 2021 19:34
antonis added a commit that referenced this pull request Feb 19, 2021
* Only enable the audio block on premium plans.

* Sync enable/disable audio block logic with iOS. Made it clearer.

* Using clear instead of setting text to empty string

* Adding textEmailAddress to EditText inputType

* Fix crash on snackbar display

* Invalidate bundler cache to fix CI failure

* Attempt to fix the CI bundler/rake cache failure again

* bundle update, to rebuild the Gemfile snapshot

* Invalidate checksum-less cache key again

* Update to bundler 2.2.10 (security fix) and bundle update google-cloud-storage to fix resolution

* Cherrypicking fix for CI bundler/rake cache failure from #14082

* Attempt to fix the CI bundler/rake cache failure again

* bundle update, to rebuild the Gemfile snapshot

* Invalidate checksum-less cache key again

* Update to bundler 2.2.10 (security fix) and bundle update google-cloud-storage to fix resolution

* Stop using multi-source mode - so that the release-toolkit gem also use the rubygems.org source

* Update Bundler + Fix Gemfile

* Updates gutenerg-mobile reference to 1.47.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants